Repository navigation
Conversation
| */ | ||
| mk_arch_send_ipi(target); | ||
| if (ret == -ENOSPC) | ||
| pr_warn_ratelimited("multikernel: IPI ring full for instance %d\n", |
There was a problem hiding this comment.
The code this replaces used printk_deferred for the ring-full case, with a comment explaining that console output reaches this path. That is still true: mktty_spawn_console_write() runs as a console ->write callback during printk flushing, holding mktty_console_lock with IRQs off, and calls multikernel_send_ipi_data(). If publish fails here, these plain printks recursively enter console flushing, which re-enters mktty_spawn_console_write() and self-deadlocks on mktty_console_lock. Ring full is most likely precisely during heavy console output, so this fires under exactly the wrong conditions. These messages should stay printk_deferred (ratelimiting can be kept via __ratelimit).
| rc = mk_arch_spawn_instance(mk_image, instance, cpu); | ||
| if (rc == 0) { | ||
| rc = mk_ipi_shared_wait_ready(instance->ipi_data, mk_id, |
There was a problem hiding this comment.
This waits up to 120 seconds while still holding the kexec lock taken by kexec_trylock() at the top of multikernel_kexec_by_id(). While a wedged spawn is timing out, every other kexec operation returns -EBUSY, and if the host panics in that window __crash_kexec() fails its trylock, so the crash kernel never boots and the vmcore is lost. Consider dropping the lock before the wait (the new MK_STATE_LOADED check already excludes concurrent exec of the same instance), or at least a much smaller default timeout.
| ret = mk_ipi_shared_validate(instance->ipi_data); | ||
| if (ret) | ||
| return ret; | ||
| if (!atomic_read_acquire(&instance->ipi_data->ready)) |
There was a problem hiding this comment.
Behavior change worth confirming: previously the host could publish into the spawn ring while the spawn booted, and the spawn drained everything after init. Now sends fail with -EAGAIN until the spawn multikernel_init (subsys_initcall) marks the downlink ready, and the callers in messaging.c and mktty do not retry, so anything sent in that window is silently dropped. If this fail-closed behavior is intended it deserves a mention in the commit message; otherwise callers need a retry, or the ready gate needs to allow queuing during boot.
| if (ret) | ||
| mk_manifest_reject_and_park(ret); | ||
| if (!atomic_read_acquire(&host_instance->ipi_data->ready) || | ||
| READ_ONCE(host_instance->ipi_data->ready_instance_id) != 0) |
There was a problem hiding this comment.
This requires the host ring ready_instance_id to be exactly 0, but a mid-level kernel acting as a host marks its receive ring with its own root_instance->id, which is nonzero for a spawn. A nested spawn therefore always parks here with -EHOSTDOWN. Nesting already has known gaps, but this turns them into a hard block at the handshake.
| @@ -1504,6 +1548,17 @@ static int __init multikernel_init(void) | |||
| return ret; | |||
| } | |||
|
|
|||
| ret = mk_ipi_shared_mark_ready(root_instance->ipi_data, | |||
There was a problem hiding this comment.
If mk_instance_restore_from_manifest() fails to allocate the root instance (its -ENOMEM path returns with root_instance still NULL and does not park), this dereferences NULL at subsys_initcall time. A NULL check turning it into a clean init failure would be safer.
3330928 to
1a984b7
Compare
|
Why This grows the vmlinux Note also that an older vmlinux with the 8-byte note now silently falls through to "entry not found" instead of a clear mismatch error like the bzImage path gives. More generally the design looks over-complicated: the 64-bit owner+index producer gate with five slot states and cancel/recover logic, and the host-side 120s readiness poll with abort-on-timeout, could both be replaced by a plain per-slot ready flag plus "reset the ring after confirm_parked", which is what the old |
1a984b7 to
4a71019
Compare
4a71019 to
e19b6b7
Compare
|
@nickolaev Both the PR description and the commit message still contain "timer calibration". Also, there is a merge conflict. |
e19b6b7 to
a7a784e
Compare
|
@nickolaev Please keep this PR to the duplex parent/child link fix (per-link rings, single producer, reset after confirm_parked, routing to the real parent) and drop the unrelated changes:
Send any of these separately if still needed. Please also update the PR title and description. |
a7a784e to
957fb0b
Compare
nickolaev
changed the title
|
I am very concerned about Publishing a kimage is a kexec job: set This function ties the two together so the endpoint can be registered at load time, and that coupling causes real problems:
None of the problems this PR fixes (single producer per ring, halt notices to the real parent, RCU handler lifetime) need anything at load time. Suggested change:
The destroy vs. load ID reuse check ( Minor: a publish failure returns without any |
Children currently publish into a shared parent inbox, while hardirq sends depend on mutex-backed instance lookup. Give each parent/child pair separate rings and serialize local producers in each direction. Use RCU for endpoint and handler lifetime. Bound dispatch to eight links and eight slots per link, with readiness scanning for fair continuation. Keep IRQ-disabled sends and polling available for PCI startup, and defer ordinary interrupt dispatch until built-in consumers register. Attach endpoints during exec and detach before publishing a stopped state. Confirm park before resetting the selected link, and reject deferred halt work from an earlier start. Keep image loading independent of transport activation. Route to the manifest's parent ID, map its halt area separately, and order publication before the APIC doorbell. Signed-off-by: Nikolay Nikolaev <nicknickolaev@gmail.com>
957fb0b to
57799ac
Compare
Give each parent/child pair separate IPI rings, serialize local producers, and route messages to the parent recorded in the manifest. Protect endpoint and handler lifetime with RCU and bound each dispatch pass to eight links and eight slots per link.
Attach the endpoint during exec and detach it on halt. Confirm park before resetting the selected link, and ignore deferred halt work from an earlier start. Loading an image does not activate transport or require assigned CPUs; sends to an inactive instance return
-ENODEV. Keep IRQ-disabled polling available for PCI startup and enable ordinary interrupt dispatch after built-in consumers register.Image loading stays as on master. Parent and child kernels must use the same transport layout; ABI negotiation, the separate load/removal race, and SR-IOV changes are outside this PR.
Validation: full x86 kernel/modules build; QEMU CPU-free loading, inactive sends, four serial load/unload cycles with resource recovery, and 128 messages each way before and after an existing-image restart, with lock/atomic checks enabled.